[BugFix] Fix minimax m2 model rotary_dim#30384
Merged
vllm-bot merged 1 commit intovllm-project:mainfrom Dec 10, 2025
Merged
Conversation
Signed-off-by: xuebi <xuebi@minimaxi.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug in the minimax_m2 model where the rotary dimension was being scaled twice. The issue stemmed from passing an already-scaled rotary_dim to the get_rope function, which would then apply the partial_rotary_factor again. The proposed fix of passing self.head_dim as the rotary_dim to get_rope is the correct approach, as it provides the unscaled base dimension, allowing get_rope to perform the scaling correctly. The change is logical, minimal, and effectively resolves the described problem.
jeejeelee
approved these changes
Dec 10, 2025
5 tasks
Isotr0py
approved these changes
Dec 10, 2025
5 tasks
Majid-Taheri
pushed a commit
to Majid-Taheri/vllm
that referenced
this pull request
Dec 23, 2025
Signed-off-by: xuebi <xuebi@minimaxi.com> Co-authored-by: xuebi <xuebi@minimaxi.com> Signed-off-by: Ubuntu <mjtaheri68@gmail.com>
dsuhinin
pushed a commit
to dsuhinin/vllm
that referenced
this pull request
Jan 21, 2026
Signed-off-by: xuebi <xuebi@minimaxi.com> Co-authored-by: xuebi <xuebi@minimaxi.com> Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
After #29966, get_rope always reads partial_rotary_factor from the configuration and performs the multiplication again, even if rotary_dim has already been scaled. This leads to the factor being applied repeatedly, incorrectly reducing the effective rotational dimension.
Another way to fix #30349.
Test Result
lm_eval --model local-completions \ --model_args base_url=http://localhost:10086/v1/completions,tokenizer=/model,model=/model \ --tasks gsm8k_cot \ --batch_size 128 \ --num_fewshot 5 local-completions (base_url=http://localhost:10086/v1/completions,tokenizer=/model,model=/model), gen_kwargs: (None), limit: None, num_fewshot: 5, batch_size: 128 | Tasks |Version| Filter |n-shot| Metric | |Value | |Stderr| |---------|------:|----------------|-----:|-----------|---|-----:|---|-----:| |gsm8k_cot| 3|flexible-extract| 5|exact_match|↑ |0.9219|± |0.0074| | | |strict-match | 5|exact_match|↑ |0.9098|± |0.0079|Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.